Update Nanosoldier to Julia 0.6 and JSON benchmarks #38

ararslan · 2017-10-25T03:46:50Z

In order to use the fancy new BenchmarkTools JSON-based serialization, we also need to update the Nanosoldier to use Julia 0.6.

I'm not intimately familiar with this code but the fact that benchmarks are loaded from JLD files as a Dict seems to be a pretty deep-seated assumption, so that will be a challenge to unravel completely.

musm · 2017-10-25T18:51:45Z

REQUIRE

@@ -1,7 +1,6 @@
-julia 0.4
+julia 0.6
 BenchmarkTools 0.0.7


Shouldn't BenchmarkTools be also updated with the new minimum after it's tagged of course

Yes. It isn't tagged yet.

jrevels · 2017-10-26T16:03:12Z

benchmarks are loaded from JLD files as a Dict seems to be a pretty deep-seated assumption, so that will be a challenge to unravel completely.

Is this the case? I think what was being saved/loaded from the JLD file is a BenchmarkGroup, i.e. the type of results here is BenchmarkGroup. That function's caller stores/passes around this BenchmarkGroup in a Dict (confusingly also named results), but that shouldn't be a problem wrt (de)serialization.

It's been a while, though...I could be misremembering 😛

ararslan · 2017-10-28T01:22:24Z

I added a commit that turns on precompilation but had to revert it because apparently GitHub.jl doesn't support it. I can try again (perhaps in a separate PR, depending on how long this is open) after JuliaWeb/GitHub.jl#93 is merged and tagged.

jrevels · 2017-10-28T14:40:59Z

src/jobs/BenchmarkJob.jl

+    loaded = BenchmarkTools.load(benchresults)[2]
+    ind = findfirst(x->isa(x, BenchmarkTools.BenchmarkGroup), loaded)
+    @assert ind > 0
+    results = loaded[ind]


Can you walk me through this? I had thought that BenchmarkTools.load(benchresults) would return results::BenchmarkGroup directly, since that what was saved via BenchmarkTools.save earlier. I'm probably misremembering something...

With the JSON format, save writes

[{"Julia":"x.y.z","BenchmarkTools":"0.2.0"},[["BenchmarkTools.BenchmarkGroup",{"tags":[...],"data":{...}}]]]

Then load makes this

[Dict("Julia"=>"x.y.z", "BenchmarkTools"=>"0.2.0"), [BenchmarkGroup(...)]]

That is, the result is a two-element array where the first is the version information and the second is an array of the deserialized values.

loaded = BenchmarkTools.load(benchresults)[2] # gets the vector of values ind = findfirst(x->isa(x, BenchmarkGroup), loaded) # figure out which of the elements is a BenchmarkGroup @assert ind > 0 # make sure one of them is results = loaded[ind] # retrieve the BenchmarkGroup

Oh. At some point in the future, we should change load to just return the second element, without the version info Dict. AFAICT there's no reason for load to expose version info to the caller. My bad, I should've caught this when reviewing JuliaCI/BenchmarkTools.jl#79. Let's ignore it for now, though, for poor Nanosoldier's sake.

We'd might as well make that change now before too many packages start depending on the current behavior.

Nah I'm just an idiot, this is already how it works.

...well that was the easiest API patch ever 😛

jrevels · 2017-10-28T14:43:27Z

src/jobs/BenchmarkJob.jl

@@ -336,7 +336,7 @@ function execute_benchmarks!(job::BenchmarkJob, whichbuild::Symbol)
                      results = run(benchmarks; verbose = true)

                      println("SAVING RESULT...")
-                      BenchmarkTools.save(\"$(benchresults)\", "results", results)
+                      BenchmarkTools.save(\"$(benchresults)\", results)


The benchresults filename extension needs to be changed from .jld to .json here.

jrevels · 2017-10-28T15:13:43Z

I added a commit that turns on precompilation

No reason not to enable precompilation here, but it wouldn't actually matter, right? IIRC the server should only ever be launched with --precompile=no --compilecache=no because we're not running on a networked filesystem, so the precompile state UUID doesn't match up correctly with the UUIDs of the Julia binaries built for each node. Maybe that's been fixed by now, though?

ararslan · 2017-10-28T21:45:30Z

Maybe that's been fixed by now, though?

I believe so, which is why I wanted to experiment with precompilation.

JeffBezanson · 2017-10-30T20:34:46Z

Bump. How are we doing here?

ararslan · 2017-10-30T21:00:23Z

So far so good. BaseBenchmarks needs to be updated, and I'm working on that, then I'll need to do some testing on Nanosoldier. This is at the top of my priority list.

jrevels · 2017-10-31T13:37:24Z

.travis.yml

@@ -3,7 +3,7 @@ language: julia
 os:
  - linux
 julia:
-  - release


- 0.6 should be added here in place of release, yeah?

In #38, `NanosoldierError` was mistakenly changed from being declared as `type` to `struct`. The object is constructed piecemeal in the benchmark job code, so the type needs to be mutable.

ararslan requested a review from jrevels October 25, 2017 03:46

musm reviewed Oct 25, 2017

View reviewed changes

ararslan force-pushed the aa/the-future branch 2 times, most recently from 3ac1e2b to 27c41b5 Compare October 28, 2017 01:21

jrevels reviewed Oct 28, 2017

View reviewed changes

ararslan force-pushed the aa/the-future branch from 27c41b5 to 5fd2c49 Compare October 28, 2017 22:49

ararslan force-pushed the aa/the-future branch from 5fd2c49 to af083d0 Compare October 30, 2017 23:27

jrevels reviewed Oct 31, 2017

View reviewed changes

ararslan added 2 commits October 31, 2017 21:46

Update Nanosoldier to Julia 0.6 and JSON benchmarks

6f8e875

Replace dependency on HttpCommon with HTTP

9e3ea12

ararslan force-pushed the aa/the-future branch from abb4f44 to 9e3ea12 Compare November 1, 2017 04:46

ararslan changed the title ~~WIP: Update Nanosoldier to Julia 0.6 and JSON benchmarks~~ Update Nanosoldier to Julia 0.6 and JSON benchmarks Nov 1, 2017

jrevels merged commit 29f098c into master Nov 1, 2017

ararslan deleted the aa/the-future branch November 1, 2017 17:54

ararslan mentioned this pull request Nov 2, 2017

Fixes + precompilation #39

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update Nanosoldier to Julia 0.6 and JSON benchmarks #38

Update Nanosoldier to Julia 0.6 and JSON benchmarks #38

ararslan commented Oct 25, 2017

musm Oct 25, 2017

ararslan Oct 25, 2017

jrevels commented Oct 26, 2017

ararslan commented Oct 28, 2017

jrevels Oct 28, 2017

ararslan Oct 28, 2017 •

edited

Loading

jrevels Oct 30, 2017

ararslan Oct 30, 2017

ararslan Oct 30, 2017

jrevels Oct 31, 2017

jrevels Oct 28, 2017

ararslan Oct 28, 2017

jrevels commented Oct 28, 2017

ararslan commented Oct 28, 2017

JeffBezanson commented Oct 30, 2017

ararslan commented Oct 30, 2017

jrevels Oct 31, 2017

ararslan Oct 31, 2017

@@ @@ -3,7 +3,7 @@ language: julia @@
               os:
                 - linux
               julia:
-                - release

Update Nanosoldier to Julia 0.6 and JSON benchmarks #38

Update Nanosoldier to Julia 0.6 and JSON benchmarks #38

Conversation

ararslan commented Oct 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrevels commented Oct 26, 2017

ararslan commented Oct 28, 2017

Choose a reason for hiding this comment

ararslan Oct 28, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jrevels commented Oct 28, 2017

ararslan commented Oct 28, 2017

JeffBezanson commented Oct 30, 2017

ararslan commented Oct 30, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ararslan Oct 28, 2017 •

edited

Loading